[OPEN] Find in Files Improvements (Part 1: Templates)#3324
[OPEN] Find in Files Improvements (Part 1: Templates)#3324dangoor merged 2 commits intoadobe:masterfrom TomMalbran:tom/find-in-files-p1
Conversation
|
Tagging open. Due to the string change, we'll wait to take this in sprint 25. Please do not merge in sprint 24. |
|
Hi @TomMalbran . Sorry we didn't get to this sooner. If you don't mind updating the pull request to the latest master, I'll take a look during sprint 27 |
ghost
assigned
There was a problem hiding this comment.
This is actually the old search-results.html, but renamed so I could use the results name for the results table.
There was a problem hiding this comment.
Is this panel only used by find in files? I'm wondering if we only need one template rather than two (the new search-results.html template get's merged in where {{SEARCH_RESULTS}} appears in this template)
There was a problem hiding this comment.
This panel is only used for find in files currently, but could eventually be used for something else later (like replace all).
This template is used to create one time only a panel to place the results in. Once it is created on start up is not used anymore. The content template is used after every search, so merging both templates would require to destroy the current panel and re-create it after each search, and once paging is added, for every new page. I think that the current solution using 2 templates is cleaner.
There was a problem hiding this comment.
OK, that seems reasonable. Thanks for the explanation!
|
@dangoor Thanks for getting this request. I just merged the code, and checked that everything stills works ok, but there were several conflicts, so I hope I didn't broke anything. |
|
Other than the one question that I have about two templates vs. one, I'm all done with review. This looks like a nice cleanup! |
[OPEN] Find in Files Improvements (Part 1: Templates)
3 participants
As I mentioned in #3151 I am going to simplify the review and test tasks by splitting the pull request into smaller steps.
For this first part I just changed the jQuery html creation with Mustache templates for the search dialog and results, and added a row selection like JSLint uses. There might be a problem with the selection color since the highlighted query and the selected row use the same color, but since the search query is also selected in the document, this might not be that big of an issue. If it is, we could change the color for one of them, or use a different color for the highlighted query when a row is selected.